Add cloud-connector as a native foremanctl feature#569
Conversation
| group: root | ||
| mode: '0755' | ||
|
|
||
| - name: Add Foreman CA to system trust store |
There was a problem hiding this comment.
Is this how we do it today? This is an anti-pattern we have been trying to avoid.
There was a problem hiding this comment.
foreman-installer does the same thing via puppet-certs:
- templates/rhsm-katello-reconfigure.erb — copies to
/etc/pki/ca-trust/source/anchors/and runsupdate-ca-trust - manifests/katello.pp — uses
trusted_ca::cato do the same
We need it here because yggdrasil-worker-forwarder is a Go binary that uses the OS trust store — no way to pass it a CA path. That said, we could move this into the certificates role so it's done once globally instead of per-feature. Would that be cleaner?
There was a problem hiding this comment.
I was apprehensive about this too. I figured we can change it later..
Claude reply above
There was a problem hiding this comment.
- templates/rhsm-katello-reconfigure.erb — copies to
/etc/pki/ca-trust/source/anchors/and runsupdate-ca-trust- manifests/katello.pp — uses
trusted_ca::cato do the same
These are both 404s because they were moved quite a bit ago. I think we need to consider udpating yggdrasil-worker-forwarder vs. starting this trend of relying on the system store. Let's phone a friend for another opinion. @evgeni ☎️
There was a problem hiding this comment.
We need to update it anyway to use DBUS (so it can run on RHEL 10), so we can probably just tack that change on there.
There was a problem hiding this comment.
https://github.com/theforeman/yggdrasil-worker-forwarder doesn't seem to use Github Issues, so I will create an internal Jira for that.
There was a problem hiding this comment.
I updated the description of https://redhat.atlassian.net/browse/SAT-27307. However, ideally I would like this PR to be merged and use the "incorrect" approach temporarily, to move things forward,.
There was a problem hiding this comment.
The problem is -- if this makes this change, and then another feature relies on it, we end up with a snowball effect of things relying on this paradigm. I think we need to come to a consensus about whether we configure the system trust with our CA or not.
There was a problem hiding this comment.
Yeah, seems my ☎️ was broken and I never seen this ping.
But I do agree with Eric, we do not want to add certs to the system trust, as
- it's not inherited by containers, so they need the explicit file passed anyway
- it makes tools we do not manage trust this CA
- it's a pattern we are trying to avoid as it makes things inconsistent (because of the above)
| protocol = "grpc" | ||
| env = [ | ||
| "FORWARDER_USER={{ cloud_connector_user }}", | ||
| "FORWARDER_PASSWORD={{ cloud_connector_password }}", |
There was a problem hiding this comment.
This is the part we still need to work out how to properly handle, that is, the authentication.
There was a problem hiding this comment.
As I see it our options are
1 - Keep things as they are here, feeding admin/changeme to rhc - Not acceptable because the admin password ends up in the worker config.toml.
2 - Create a service user with limited permissions - Doable in the existing PRs here and in foreman_rh_cloud.
3 - Create a service user + personal access token and use that, same as the previous architecture - FAM doesn't have a module for personal access tokens, so this would be a fair bit of work
I think Option 2 seems promising and am pushing that update now.
There was a problem hiding this comment.
@ehelms Updated to use OAuth, but we still need the service user because yggdrasil-worker-forwarder only supports HTTP basic auth, and doesn't support OAuth.
|
Pushed updates addressing review feedback (1bfe640):
|
908e8b1 to
95a9cc9
Compare
2092035 to
84335ce
Compare
|
rebased & fixed conflicts |
qcjames53
left a comment
There was a problem hiding this comment.
I'm not enough in the loop to give this a full review but had a few comments that might be helpful. Consider this 80% good points 20% ass-talking. Here's what I was able to test directly:
- The iop conflict and missing consumer cert checks both work on the positive and negative cases (used manual subscription manager commands for the latter).
- The
yggdrasil-worker-forwardercheck works in the negative (was unable to figure out package install on centos9). - The cloud connector role works.
Beyond these tests, I have questions about the whole approach of cloud connector even after reading the planning docs and such.
- How are we splitting RHEL and Centos 9 deploys? I think the differentiation needs to be clearer since (best I can tell)
yggdrasil-worker-forwarderis RHEL only. If the packaged container images are centos9 and the rpms needed for cloud connector are RHEL-only, that seems like a big problem to me! deploy-devneeds to work with cloud connector. At minimum a procedure but ideally out of the box. I think we really need a procedure for setting up a centos9 deploy-dev environment to build/test cloud connector. If that's not possible, then we need a good procedure for setting up a RHEL environment as deploy-dev.- If it's possible to pass in subscription manager credentials safely, I would greatly prefer if we could deploy an environment with cloud connector immediately instead of requiring manual registration for the quadlet VM, THEN adding the feature. I understand this won't be an issue for customers but this would be HUGE for testing purposes.
I have to gracefully bow out of reviewing this for a few reasons: capacity + artemis stuff + this baby could come any day! Sorry to not complete testing but I put in an honest effort.
| The system must be registered with subscription-manager. | ||
|
|
||
| - name: Check that yggdrasil-worker-forwarder package is available | ||
| ansible.builtin.command: dnf info yggdrasil-worker-forwarder |
There was a problem hiding this comment.
yggdrasil-worker-forwarder - Is this available in centos9? Where is it shipped? I know it won't be a problem for shipping pre-built images but for dev environments and such we should really document a pathway for getting this installed.
There was a problem hiding this comment.
I checked and it's in https://yum.theforeman.org/plugins/3.19/el9/x86_64/
| - name: Read client ID from CN of consumer certificate | ||
| ansible.builtin.command: openssl x509 -in /etc/pki/consumer/cert.pem -subject -noout | ||
| register: cloud_connector_cert_output | ||
| changed_when: false |
There was a problem hiding this comment.
Same as above but opposite. Idempotency?
There was a problem hiding this comment.
This is correct — changed_when: false means "this command never changes anything," which is right for a read-only openssl x509 call. Standard Ansible pattern for info-gathering tasks.
|
I'll respond with thoughts off the top of my head :)
In order for cloud connector to work you need available on your base system:
In the previous architecture, these weren't even listed as prerequisites, even in upstream docs. Downstream, you'd already have all of these since the Satellite system must be registered and have those repos enabled. But upstream, anything can happen. It seems this is a documentation gap. cc @Lennonka
I was testing with deploy-dev on RHEL (with |
|
@qcjames53 Addressed 2 comments and replied to the third |
|
Tested cloud connector feature in development mode. All core functionality works correctly. Found 3 necessary configuration changes for development environment. 🔍 Testing MethodologyEnvironment: Development mode using
🔗 Related/Dependent PRsForeman RH Cloud Plugin
Foremanctl Branch Preservation
Note: These PRs should be merged before or alongside this cloud connector feature PR. ✅ What Works1. Cloud Connector Deployment
Note: The cloud connector role does not install packages - it assumes 2. Service User AuthenticationCreated service user 3. API Endpoint VerificationManual test of Request: curl -u cloud_connector_user:$PASSWORD \
-X POST \
http://localhost:3000/api/v2/rh_cloud/cloud_request \
-H 'Content-Type: application/json' \
-d '{"directive":"foreman_rh_cloud", ...}'Result: 4. Configuration Validation
🔧 Required Code ChangesFound 3 necessary changes for development environment support: Change 1: Development URL ConfigurationFile: Issue: Development uses Rails dev server on port 3000, not containerized Foreman Fix: vars:
cloud_connector_url: "http://localhost:3000"
cloud_connector_admin_user: "{{ foreman_development_admin_user }}"
cloud_connector_admin_password: "{{ foreman_development_admin_password }}"Why: Points worker to the correct development Rails server endpoint Change 2: Disable Certificate Validation for HTTPFile: Issue: Development uses HTTP (no SSL), certificate validation fails Fix: - name: Create cloud connector service user
theforeman.foreman.user:
# ... existing parameters ...
validate_certs: false # <-- Add thisWhy: HTTP connections don't have valid certificates in development Change 3: Make CA Certificate Path OptionalFile: Issue: No CA certificate exists for HTTP connections, undefined variable error Fix: - name: Create cloud connector service user
theforeman.foreman.user:
# ... existing parameters ...
ca_path: "{{ foreman_ca_certificate | default(omit) }}" # <-- Change thisWhy: Prevents Ansible error when
|
|
Re: Change 1 - cloud_connector_url defaults to foreman_url which is https://{{ fqdn }}, Change 2 is not necessary unless I were to do Change 1, which I think is not needed for the reason above. Updating now with Change 3. |
|
Btw, Our role does install the packages — line 2-7 of tasks/main.yaml: - name: Install rhc and yggdrasil-worker-forwarder
ansible.builtin.package:
name:
- rhc
- yggdrasil-worker-forwarder |
012188d to
9b8b99d
Compare
|
We decided that foremanctl should not rely on the system CA trust store; rather, the applications consuming the certificate must be configured to point to the certificate directly. This means the consuming application (yggdrasil in this case) must be able to be configured in this way. Converting to draft while we wait for that to be implemented in yggdrasil. |
| oauth1_consumer_secret: "{{ foreman_oauth_consumer_secret }}" | ||
| ca_path: "{{ foreman_ca_certificate | default(omit) }}" | ||
|
|
||
| - name: Announce to Sources |
There was a problem hiding this comment.
What does announce to sources do?
There was a problem hiding this comment.
It's an API call made to the cloud to identify this Foreman instance as a source in a service that was previously called Sources - I'm not sure what the name of the service is now. My understanding is that it's needed in order for cloud remediations to show your registered host as "connected"
|
|
||
| - name: Enable automatic inventory upload | ||
| theforeman.foreman.setting: | ||
| name: allow_auto_inventory_upload |
There was a problem hiding this comment.
Why do we have to set this? Is the default value false?
There was a problem hiding this comment.
Setting up cloud connector has always required turning on this setting, and in the previous architecture there was a message in the UI to that effect. The default is true, but the user may have turned it off and then tried to enable cloud connector.
There was a problem hiding this comment.
cc @ShimShtein in case you can add more color here
4eb0858 to
94c83d8
Compare
|
|
this also required a change to theforeman/foreman_rh_cloud#1214, so be sure to pull that too |
Re-implements the upstream satellite_operations.cloud_connector role natively in foremanctl so users can enable it via: foremanctl deploy --add-feature cloud-connector Feature overview: - Installs rhc and yggdrasil-worker-forwarder packages - Templates the worker config with FORWARDER_CA_FILE for explicit CA trust (requires yggdrasil-worker-forwarder >= 0.0.4) - Creates a dedicated service user with limited dispatch_cloud_requests permission for runtime worker authentication - Uses OAuth for all Foreman API calls during setup (role, user, settings), matching the pattern used by other foremanctl roles - Adds a reusable foremanctl_api custom module for OAuth-authenticated API calls where no FAM module exists - Sets rhc_instance_id and enables automatic inventory upload - Announces to Sources via the foreman_rh_cloud API - Optional HTTP proxy support via --cloud-connector-http-proxy - Pre-checks validate consumer certificate and iop mutual exclusion - Works with both foremanctl deploy and forge deploy-dev Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4ff51ad to
daf9b90
Compare
Summary
satellite_operations.cloud_connectorrole natively in foremanctl (SAT-45966 / SAT-44641)cloud-connectorfeature gated behind--add-feature cloud-connector, withrh-clouddependencyrhcandyggdrasil-worker-forwarder, configures the worker, startsrhcd, setsrhc_instance_idvia the Foreman API, and announces to Sourcesforemanctl deployandforge deploy-devcloud_connector_userservice user with limiteddispatch_cloud_requestspermission for runtime worker authenticationforemanctl_apicustom module for OAuth-authenticated API calls where no FAM module existsiopfeature--cloud-connector-http-proxyflag for environments without direct internet accessCompanion PRs:
theforeman/foreman_rh_cloud#1214 (adds the
announce_to_sourcesAPI endpoint)theforeman/yggdrasil-worker-forwarder#5 (allows specifying a certificate to use)
Test plan
foremanctl deploy --add-feature cloud-connectorcompletes successfullyforemanctl features --list-enabledincludescloud-connectorsystemctl status rhcdshows active/running/etc/rhc/workers/foreman_rh_cloud.tomlhas correct content with service user credentialshammer settings info --name rhc_instance_idshows the consumer cert CNhammer user info --login cloud_connector_usershows the service user with Cloud Connector roleforemanctl deployis idempotentforge deploy-dev --add-feature cloud-connectorcompletes successfullycloud-connectorandiopfails early with a clear error🤖 Generated with Claude Code